txt2html: update to 3.0 #27115

srirangav · 2024-12-17T18:20:19Z

Description

Update txt2html port to version 3.0 (the current version of the port apparently does not work with perl 5.30 and newer, as recently reported on macports-users).

Closes: https://trac.macports.org/ticket/71566

Type(s)

bugfix
enhancement
security fix

Tested on

macOS 13.7.1 22H221 arm64
Xcode 14.3.1 14E300c

Verification

Have you

followed our Commit Message Guidelines?
squashed and minimized your commits?
checked that there aren't other open pull requests for the same change?
referenced existing tickets on Trac with full URL in commit message?
checked your Portfile with port lint?
tried existing tests with sudo port test?
tried a full install with sudo port -vst install?
tested basic functionality of all binary files?
checked that the Portfile's most important variants haven't been broken?

Dave-Allured · 2024-12-17T22:46:28Z

Please add this line to the commit message.

Closes: https://trac.macports.org/ticket/71566

ryandesign

referenced existing tickets on Trac with full URL in commit message?

Please add to the commit message the line:

Closes: https://trac.macports.org/ticket/71566

ryandesign · 2024-12-18T00:30:36Z

textproc/txt2html/Portfile

+configure   { system "cd ${worksrcpath} && \
+                     '${perl5.bin}' './Build.PL' '--install_base' '${prefix}'" }
+
+build       { system "cd ${worksrcpath} && \
+                     '${perl5.bin}' '${worksrcpath}/Build'" }
+
+destroot    { system "cd ${worksrcpath} && \
+                     '${perl5.bin}' '${worksrcpath}/Build' 'install' \
+                     'destdir=${destroot}'" }


You shouldn't need to override these phases with custom commands; the standard method used by the perl5 portgroup should work and will add a variety of arguments and environment variables that enable MacPorts features users expect.

You should almost never need to use cd in a system command; use system's -W flag instead.

You shouldn't need to override these phases with custom commands; the standard method used by the perl5 portgroup should work and will add a variety of arguments and environment variables that enable MacPorts features users expect.

You should almost never need to use cd in a system command; use system's -W flag instead.

I have updated the Portfile to use the default Perl5 build system as much as possible.

I couldn't figure out how to have the default Perl5 build system supply the --install-base argument to the Build.PL script, so I kept the configure{} phase, but I used the system -W command as you suggested.

Also, I kept a modified version of the post-destroot{} phase to fix warnings about the man pages and perl5 libraries not being installed in the correct /opt/local directories.

Please let me know if I can simplify this further.

Thanks!

ryandesign · 2024-12-18T00:31:39Z

textproc/txt2html/Portfile

+post-destroot { system "mkdir '${destroot}/${prefix}/lib/perl5/${perl5.major}' && \
+                        mv '${destroot}/${prefix}/lib/perl5/HTML' \
+                           '${destroot}/${prefix}/lib/perl5/${perl5.major}'"
+                system "mv '${destroot}/${prefix}/man/man1/txt2html.1pm' \
+                           '${destroot}/${prefix}/share/man/man1/'"
+                system "mv '${destroot}/${prefix}/man/man3/HTML::TextToHTML.3pm' \
+                           '${destroot}/${prefix}/share/man/man3/'" }


Don't use system for something Tcl can do natively. (All of these commands can be done natively.)

The value of ${prefix} already begins with a / so it is an error to literally precede ${prefix} with another /.

I have made these changes.

ryandesign · 2024-12-18T00:33:01Z

textproc/txt2html/Portfile

-		  system "install -m 644 ${worksrcpath}/* \
-		    ${destroot}${prefix}/share/doc/${name}"
-		  system "rm ${destroot}${prefix}/share/doc/${name}/${name}.*" }
+# -*- coding: utf-8; mode: tcl; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- vim:fenc=utf-8:ft=tcl:et:sw=4:ts=4:sts=4


You've changed the whitespace of the entire file in addition to making functional changes, making it very hard to identify and review the functional changes. Please separate whitespace-only changes into a separate commit, if they must be done.

I reverted the whitespace changes and only included functional changes in my latest commit. Thanks.

ryandesign · 2024-12-18T00:36:23Z

textproc/txt2html/Portfile

+categories          textproc
+license             BSD
+maintainers         nomaintainer
+homepage            ${github.homepage}


You can remove this line. github.setup does this for you automatically.

Suggested change

homepage ${github.homepage}

I have removed this line. Thanks.

ryandesign · 2024-12-18T01:54:46Z

textproc/txt2html/Portfile

+checksums           md5     037f5fdfae92181d2bb7abfb443f6949 \
+                    rmd160  fd91190626dbf6727fac8b027b8e8fa349168450 \


The MD5 algorithm is obsolete and there's no value to including it here.

Suggested change

checksums md5 037f5fdfae92181d2bb7abfb443f6949 \

rmd160 fd91190626dbf6727fac8b027b8e8fa349168450 \

checksums rmd160 fd91190626dbf6727fac8b027b8e8fa349168450 \

I have removed md5 from the checksums. Thanks.

srirangav · 2024-12-18T19:19:08Z

Please add this line to the commit message.

Closes: https://trac.macports.org/ticket/71566

I have updated the commit message and the comment above with this.

srirangav · 2024-12-18T19:20:04Z

referenced existing tickets on Trac with full URL in commit message?

Please add to the commit message the line:
Closes: https://trac.macports.org/ticket/71566

I have checked this box in the comment above, and have updated the comment and the commit message.

textproc/txt2html/Portfile

reneeotten · 2024-12-28T15:30:06Z

textproc/txt2html/Portfile

@@ -1,12 +1,12 @@
 PortSystem 1.0


please add another commit with non-functional changes where you fix the whitespace issues (i.e., with spaces in multiples of four).

Once this pull request is merged, I will create a new pull request that fixes the whitespace issues and updates the revision to 1.

No, there is no need to increase the "recision" but you'll need to correct the whitespace formatting before we'll merge this.

Hi @reneeotten,

Would you like me to make a separate commit on top of the one I've already made that includes the whitespace changes? If so, I would be happy to do that.

I'm asking, because normally I just amend my existing commit when fixing issues.

Thanks,

-ranga

You can amend for now. What I meant to say is that normally you would separate functional changes and whitespace changes in two separate commits. However, here you introduce wrong alignment while doing the update so it's fine to correct that and then amend your commit.

Hi, I have changed the space on this line to a tab to match the existing formatting. Let me know if I should revert this and instead update the lines I added / changed to use the usual formatting.

the whole Portfile should adhere to the correct whitespace....

Hi @reneeotten,

I have pushed a new commit with whitespace changed. Please let me know if this commit is okay.

Thanks,

-ranga

textproc/txt2html/Portfile

Closes: https://trac.macports.org/ticket/71566

macportsbot added type: bugfix type: update maintainer: none labels Dec 17, 2024

srirangav force-pushed the txt2html-2.51 branch from afde60b to 9d7b3ba Compare December 17, 2024 21:13

srirangav changed the title ~~txt2html: update to 2.51~~ txt2html: update to 3.0 Dec 17, 2024

ryandesign requested changes Dec 18, 2024

View reviewed changes

ryandesign reviewed Dec 18, 2024

View reviewed changes

srirangav force-pushed the txt2html-2.51 branch from 9d7b3ba to f1d3efd Compare December 18, 2024 19:17

reneeotten requested changes Dec 28, 2024

View reviewed changes

srirangav force-pushed the txt2html-2.51 branch from f1d3efd to bb5f801 Compare January 1, 2025 06:24

srirangav force-pushed the txt2html-2.51 branch from bb5f801 to 1aaf21b Compare January 10, 2025 02:28

txt2html: update to 3.0

0ca0c14

Closes: https://trac.macports.org/ticket/71566

srirangav force-pushed the txt2html-2.51 branch from 1aaf21b to 0ca0c14 Compare January 16, 2025 07:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

txt2html: update to 3.0 #27115

txt2html: update to 3.0 #27115

srirangav commented Dec 17, 2024 •

edited

Loading

Dave-Allured commented Dec 17, 2024

ryandesign left a comment

ryandesign Dec 18, 2024

srirangav Dec 18, 2024

ryandesign Dec 18, 2024 •

edited

Loading

srirangav Dec 18, 2024

ryandesign Dec 18, 2024

srirangav Dec 18, 2024

ryandesign Dec 18, 2024

srirangav Dec 18, 2024

ryandesign Dec 18, 2024

srirangav Dec 18, 2024

srirangav commented Dec 18, 2024

srirangav commented Dec 18, 2024

reneeotten Dec 28, 2024

srirangav Jan 1, 2025

reneeotten Jan 9, 2025

srirangav Jan 9, 2025

reneeotten Jan 10, 2025

srirangav Jan 10, 2025

reneeotten Jan 10, 2025

srirangav Jan 16, 2025

		checksums md5 037f5fdfae92181d2bb7abfb443f6949 \
		rmd160 fd91190626dbf6727fac8b027b8e8fa349168450 \

	checksums md5 037f5fdfae92181d2bb7abfb443f6949 \
	rmd160 fd91190626dbf6727fac8b027b8e8fa349168450 \
	checksums rmd160 fd91190626dbf6727fac8b027b8e8fa349168450 \

txt2html: update to 3.0 #27115

Are you sure you want to change the base?

txt2html: update to 3.0 #27115

Conversation

srirangav commented Dec 17, 2024 • edited Loading

Description

Type(s)

Tested on

Verification

Dave-Allured commented Dec 17, 2024

ryandesign left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryandesign Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srirangav commented Dec 18, 2024

srirangav commented Dec 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srirangav commented Dec 17, 2024 •

edited

Loading

ryandesign Dec 18, 2024 •

edited

Loading